fix: accept a negative Set-Cookie Max-Age attribute - #5571
Conversation
Signed-off-by: arshiya tabasum <arshi@bugqore.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5571 +/- ##
==========================================
- Coverage 93.47% 93.34% -0.13%
==========================================
Files 110 110
Lines 37560 38778 +1218
==========================================
+ Hits 35108 36197 +1089
- Misses 2452 2581 +129 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
|
||
| // 2. If the remainder of attribute-value contains a non-DIGIT | ||
| // character, ignore the cookie-av. | ||
| if (!/^\d+$/.test(attributeValue)) { |
There was a problem hiding this comment.
Please make the following changes to ensure closer adherence to the specifications.
| if (!/^\d+$/.test(attributeValue)) { | |
| if (/[^\d]/.test(attributeValue.slice(1))) { |
There was a problem hiding this comment.
Good call, that reads much closer to the spec. I took the remainder check as suggested and also tightened step 1 to the current 6265bis wording (a "-" followed by a DIGIT). Without that, slice(1) lets a bare Max-Age=- and an empty value slip past step 2 and parse to NaN/0, so validating the sign up front keeps them ignored while step 2 stays the clean remainder check you wanted. The existing edge cases and the -1 case all still pass. Pushed in 4403160.
Signed-off-by: arshiya tabasum <arshi@bugqore.com>
| // 1. If the first character of the attribute-value is neither a DIGIT, | ||
| // nor a "-" character followed by a DIGIT, ignore the cookie-av. |
There was a problem hiding this comment.
Could you please revert this change? This comment block is intended to preserve the exact wording from the specification.
There was a problem hiding this comment.
Reverted, the comment block is back to the original spec wording. The code still does the sign-followed-by-digit check but the comment stays verbatim.
Signed-off-by: arshiya tabasum <arshi@bugqore.com>
3 participants
This relates to...
N/A
Rationale
parseUnparsedAttributesinlib/web/cookies/parse.jsvalidates theMax-Ageattribute-value with/^\d+$/, applied to the whole value rather than to the remainder after the sign. Any negativeMax-Agetherefore fails the check and the attribute is dropped from the parsed cookie:RFC 6265bis section 5.6.2 is explicit that a leading
-is allowed:The step-1 check at line 193 already accepts
-as a first character, and the last step above only has meaning if negative values reach it, so the intent in the surrounding code is the spec behaviour.Max-Age=-1is the usual way a server expires a cookie, and today that signal is silently lost bygetSetCookies().The generation side is deliberately left alone:
validateCookieMaxAgestill rejects negatives, which matches themax-age-av = "Max-Age=" non-zero-digit *DIGITgrammar a server must emit. Parsing is permissive, serialising stays strict.Changes
Relax the digit check to
/^-?\d+$/so a single leading sign is accepted and a non-DIGIT remainder is still ignored.Max-Age=-,--1,-1a,+1and the empty value all stay ignored, and positive values are unchanged.Features
N/A
Bug Fixes
Set-Cookie: ...; Max-Age=-1now parses tomaxAge: -1instead of dropping the attribute.Breaking Changes and Deprecations
N/A
Status